utils + azure: vscode.dev migration work #1401
Conversation
* T2 Only * Changes to move towards T2 Co-authored-by: Megan Mott <[email protected]>
* Edit config file to target webworker * Minor edits * Support caller-supplied fallbacks * Changes to dev webpack config * Add a comment
* Use default pipelines and add/remove policies as needed * Revert export changes * Bump version
azure/test/request.test.ts
Outdated
|
|
||
| test('operationSpec with expected error', async () => { | ||
| testResponses = [{ status: 404, body: 'oops' }]; | ||
| //test('operationSpec with unexpected error', async () => { |
There was a problem hiding this comment.
I'm not sure if we're still using operationSpec errors because it was our StatusCodePolicy pipeline controlling that. There are still some pipelines that I need to investigate into adding back in.
| @@ -211,6 +209,10 @@ export function getDefaultWebpackConfig(options: DefaultWebpackOptions): webpack | |||
| "async_hooks": false, | |||
| "child_process": false, | |||
| "fs": false, | |||
| 'html-to-text': false, | |||
There was a problem hiding this comment.
This package is used to parse errors. I'm not sure how to really test it (because I don't see html errors often) but it's wrapped in a try/catch so I don't think that this should cause any exceptions to be thrown in a vscode.dev environment.
There was a problem hiding this comment.
Hasn't this one given us CG alerts before? Just to parse errors feels like a pretty flimsy reason to keep it...
There was a problem hiding this comment.
It's only used to parse error messages in case the error message is an html document, which seems somewhat common for Azure errors. See #360
There was a problem hiding this comment.
Also, I couldn't quickly find any alternatives that are browser compatible, so 🤷
There was a problem hiding this comment.
Oh the irony...can't find a browser-compatible package to parse HTML
| /** | ||
| * The Azure SDK will only throw errors for bad status codes if it has an "operationSpec", but none of our "generic" requests will have that | ||
| */ | ||
| class StatusCodePolicy implements PipelinePolicy { |
There was a problem hiding this comment.
This is basically what we had before for the following reason:
The Azure SDK will only throw errors for bad status codes if it has an "operationSpec", but none of our "generic" requests will have that
OperationSpec isn't something that you can pass into the request anymore, but I believe it's built internally into the SDK. If the SDK call errors out, it throws the error during the pipeline. However, with generic requests, it will accept any status code as a resolved response, so we need to be checking the error and throwing it ourselves.
azure/test/request.test.ts
Outdated
| }); | ||
|
|
||
| test('ECONNRESET', async () => { | ||
| await assertThrowsAsync(async () => await sendTestRequest(res => res.destroy()), /socket hang up/i); | ||
| }); | ||
|
|
||
| // test('operationSpec with unexpected error', async () => { |
There was a problem hiding this comment.
As I mentioned, we don't have operationSpec as a property anymore, so I removed these tests. The errors above basically test the same thing, which was the StatusCodePolicy
|
Merge conflicts? |
|
Okaaay, that is my last commit. Stupid version changes-- why can't git figure it out? |
|
Need to release dev -> utils -> azure in that order to make fix the linting errors |
TODO: There's still some commented out code that I need to fix before we can merge this, just getting the PR up